Conversation
This comment has been minimized.
This comment has been minimized.
178d259 to
abd6031
Compare
This comment has been minimized.
This comment has been minimized.
abd6031 to
3102edf
Compare
This comment has been minimized.
This comment has been minimized.
3102edf to
c206a47
Compare
self appear at the end of any paths in importsself at the end of any paths in imports
This comment has been minimized.
This comment has been minimized.
c206a47 to
d7a8a0a
Compare
d7a8a0a to
d845289
Compare
This comment has been minimized.
This comment has been minimized.
d845289 to
77169f5
Compare
This comment has been minimized.
This comment has been minimized.
77169f5 to
ce8d16a
Compare
This comment has been minimized.
This comment has been minimized.
b4d62d4 to
9ac9e00
Compare
This comment has been minimized.
This comment has been minimized.
9ac9e00 to
39dc3a0
Compare
|
It looks like #146972 (comment) suggests to support trailing However, if we are doing it, then I think it's time to abandon the whole " |
Semantics of trailing But what will trailing |
| ident = Ident::new(source.ident.name, self_span); | ||
| } | ||
| } | ||
| Ident::new(parent.ident.name, self_span) |
There was a problem hiding this comment.
So things like foo::self::self are still unsupported and will be reported by UnnamedImport.
Seems fine for a start.
There was a problem hiding this comment.
Also, use foo::self::self as name is probably supported now, right?
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Updated, use foo::self::self; is allowed.
| let mut span = binding_span; | ||
| match import.kind { | ||
| ImportKind::Single { type_ns_only: true, .. } => { | ||
| suggestion = Some(format!("self as {suggested_name}")) |
There was a problem hiding this comment.
What is this change responsible for?
There was a problem hiding this comment.
Without this, use foo::self; will have wrong suggestion use self as other_foo;. With this we will get use foo::self as other_foo; correctly
| return Ok(module.self_decl.unwrap()); | ||
| if ns == TypeNS { | ||
| if ident.name == kw::SelfLower { | ||
| return Ok(module.self_decl.unwrap()); |
There was a problem hiding this comment.
Hmm, why doesn't this enable paths like foo::self in non-import positions?
Is there some separate check in resolve_path or similar reporting an error for this case?
There was a problem hiding this comment.
Yes, resolve_path_with_ribs will check any path-kw used in the middle
There was a problem hiding this comment.
Seems we can support foo::self in non-import paths if we remove that check for self at last position.
There was a problem hiding this comment.
Updated, foo::self is allowed in non-import positions now.
b1d5b46 to
31857aa
Compare
self in import pathsself in paths
|
(I just hope force pushes won't break the perf run.) |
This comment has been minimized.
This comment has been minimized.
31857aa to
8552939
Compare
|
I will remove |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (a35a6e8): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary -7.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 480.701s -> 480.064s (-0.13%) |
…-self, r=JonathanBrouwer Remove redundant self usages Extracted from rust-lang#152996. r? petrochenkov
|
Cool, seems the perf run shows no perf regression, so maybe I could remove the |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
47d969d to
1697857
Compare
251e4e3 to
cd94176
Compare
…-self, r=JonathanBrouwer Remove redundant self usages Extracted from rust-lang#152996. r? petrochenkov
|
@rustbot ready |
View all comments
As a follow-up PR to #146972, after this PR:
selfcan appear in paths (as the consensus in Support importing path-segment keyword with renaming #146972 (comment))use ...::self [as target];will be equivalent touse ...::{self [as target]};struct S {}; use S::{self as Other};will be rejectedThis PR used to add a new lint
redundant_self, which would lintuse ...::self [as target];anduse ...::{self [as target]};, and the last commit fixes all warnings emitted by this lint.But this lint and clippy lint unnecessary_self_imports have some overlap. And
use std::io::self;is not equivalent touse std::ioin fact for now, the new lint will also cause the following known issue:So I removed this lint, and I think what it does should be done by extending the clippy lint
unnecessary_self_imports.r? petrochenkov